Skip to content

feat(team_participant): reorganize former members with subheaders in Former tab#7230

Draft
Eetwalt wants to merge 4 commits intomainfrom
tp-staff-former
Draft

feat(team_participant): reorganize former members with subheaders in Former tab#7230
Eetwalt wants to merge 4 commits intomainfrom
tp-staff-former

Conversation

@Eetwalt
Copy link
Collaborator

@Eetwalt Eetwalt commented Mar 10, 2026

Summary

Resolves #7101. Reorganizes how former staff and former players are displayed by moving them all to the "Former" tab with clear subheaders:

  • "Players" subheader - Lists former players
  • "Staff" subheader - Lists former staff members

This provides better organization and clarity compared to displaying former staff in the Staff tab with strikethrough styling.

Key Changes

  • Former staff members now appear in the Former tab under the "Staff" subheader
  • Former players appear in the Former tab under the "Players" subheader
  • Subheaders only display when their section has entries
  • Staff tab now shows only current/active staff members
  • Removed strikethrough styling - the tab context makes the status clear

How did you test this change?

dev (tsf2) + devtools

image

@Eetwalt Eetwalt requested review from a team as code owners March 10, 2026 08:42
Copilot AI review requested due to automatic review settings March 10, 2026 08:42
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adjusts TeamParticipants wiki parsing to avoid overwriting an explicitly provided participant type (e.g., former) when the participant also has a staff role, fixing incorrect tab placement in the roster UI.

Changes:

  • Update staff auto-assignment logic to trigger only when type is the default 'player', preserving explicit type values.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Collaborator

@mbergen mbergen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has major implications on storage.
https://github.com/Liquipedia/Lua-Modules/blob/main/lua/wikis/commons/TeamParticipants/Repository.lua
will use the type set here to determine the individualprizemoney, and furthermore
https://github.com/Liquipedia/Lua-Modules/blob/main/lua/wikis/commons/Opponent.lua#L566
uses the type to determine whether to store someone as player or staff.

When former staff does not get the correct type here, it will count as player in the mentioned locations and store bad data.
This also affects LPDB/API consumers downstream.

@mbergen
Copy link
Collaborator

mbergen commented Mar 10, 2026

In addition, in my personal opinion, all tabs that are not staff should be reserved for players - that should be the main thing someone is interested in.
(Perhaps the exception to this is CS where the coach is treated as essential part of the roster and would also show on the main tab)

So i'd rather fix the display on the staff tab for former staff, but not move them to another tab.

@Eetwalt
Copy link
Collaborator Author

Eetwalt commented Mar 10, 2026

Ah right, major overlook from my part

This has major implications on storage. https://github.com/Liquipedia/Lua-Modules/blob/main/lua/wikis/commons/TeamParticipants/Repository.lua will use the type set here to determine the individualprizemoney, and furthermore https://github.com/Liquipedia/Lua-Modules/blob/main/lua/wikis/commons/Opponent.lua#L566 uses the type to determine whether to store someone as player or staff.

When former staff does not get the correct type here, it will count as player in the mentioned locations and store bad data. This also affects LPDB/API consumers downstream.

Oh, didn't realize 'former' was reserved for players only. In that case yes, this approach doesn't work. Drafting the PR and thinking of alternative solutions.

Also will think if we should keep the former staff in staff tab but have a distinction there with label / strikethrough or something.

@Eetwalt Eetwalt marked this pull request as draft March 10, 2026 09:18
@Eetwalt Eetwalt changed the title fix(team_participant): preserve explicit type when assigning staff roles feat(team_participant): reorganize former members with subheaders in Former tab Mar 10, 2026
@Eetwalt Eetwalt marked this pull request as ready for review March 10, 2026 12:39
@Eetwalt Eetwalt requested a review from mbergen March 10, 2026 12:39
@Eetwalt
Copy link
Collaborator Author

Eetwalt commented Mar 10, 2026

In addition, in my personal opinion, all tabs that are not staff should be reserved for players - that should be the main thing someone is interested in. (Perhaps the exception to this is CS where the coach is treated as essential part of the roster and would also show on the main tab)

So i'd rather fix the display on the staff tab for former staff, but not move them to another tab.

Discussed with product, decided it would be best to show both Players and Staff under 'Former' but have subheaders above them so there's no confusion.

@mbergen
Copy link
Collaborator

mbergen commented Mar 10, 2026

The implementation currently in this PR seems non-functional, it is still depending on the type being former while it will only be staff, no?

@mbergen
Copy link
Collaborator

mbergen commented Mar 10, 2026

Tbh, ideally we would differentiate between type and status:

  • type can be player|staff
  • status can be active|former|substitute (although substitute and former could both happen i guess, so maybe substitute needs to be also separated into a flag?)

@Eetwalt

This comment was marked as outdated.

@Eetwalt Eetwalt marked this pull request as draft March 10, 2026 14:29
@Eetwalt
Copy link
Collaborator Author

Eetwalt commented Mar 10, 2026

The implementation currently in this PR seems non-functional, it is still depending on the type being former while it will only be staff, no?

true, had old changes in the dev env as you pointed out. back to the drawing board

@Eetwalt Eetwalt marked this pull request as ready for review March 10, 2026 14:59
Comment on lines +204 to +210
if tabTypeEnum == TAB_ENUM.STAFF then
return personType == 'staff'
elseif tabTypeEnum == TAB_ENUM.FORMER then
return player.extradata.isFormer
else
return PERSON_TYPE_TO_TAB[personType] == tabTypeEnum
end
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if tabTypeEnum == TAB_ENUM.STAFF then
return personType == 'staff'
elseif tabTypeEnum == TAB_ENUM.FORMER then
return player.extradata.isFormer
else
return PERSON_TYPE_TO_TAB[personType] == tabTypeEnum
end
if player.extradata.isFormer then
return tabTypeEnum == TAB_ENUM.former
else
return PERSON_TYPE_TO_TAB[personType] == tabTypeEnum
end

To avoid former staff from appearing in both former and staff while reducing "duplicate" code

Comment on lines +115 to +120
local formerPlayers = Array.filter(players, function(player)
return player.extradata.isFormer and player.extradata.type ~= 'staff'
end)
local formerStaff = Array.filter(players, function(player)
return player.extradata.isFormer and player.extradata.type == 'staff'
end)
Copy link
Collaborator

@mbergen mbergen Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to avoid doing this for all tabs, and only do it for the former tab

@Eetwalt Eetwalt marked this pull request as draft March 10, 2026 15:12
@mbergen
Copy link
Collaborator

mbergen commented Mar 10, 2026

In general: I still don't think its a great idea to have type and status as one.
Right now, you can mark someone as staff while not having a staff role assigned (for whatever reasons)
With this, you can mark them as former, but that would mean you can't at the same time mark them as staff (unless they are auto-marked as staff due to a role), again leading to implications on storage.

@Eetwalt
Copy link
Collaborator Author

Eetwalt commented Mar 10, 2026

In general: I still don't think its a great idea to have type and status as one. Right now, you can mark someone as staff while not having a staff role assigned (for whatever reasons) With this, you can mark them as former, but that would mean you can't at the same time mark them as staff (unless they are auto-marked as staff due to a role), again leading to implications on storage.

Yeah.. I'll discuss this with Rath later. Not happy with it either. Seems like with this non-former staff members don't render so putting it to draft again. Thanks for the patience, seems like we need a bigger rework..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TeamParticipants not assigning staff to former properly

3 participants